Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added examples as a subdirectory #706

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Conversation

taeh98
Copy link
Collaborator

@taeh98 taeh98 commented Sep 5, 2023

@hossein-zare I've added the examples in the rndp-examples subdirectory and updated the README to mention it. There are the common zIndex/overlapping issues, but it otherwise works. It should only take a couple of minutes to look at it, so I'll leave this PR open for a little while in case you want to. If you do, please let me know if you're happy with the new examples or if you'd suggest any changes. Thanks!

@taeh98 taeh98 force-pushed the examples branch 2 times, most recently from 67464a6 to 1b4777f Compare September 5, 2023 18:02
@hossein-zare
Copy link
Owner

Please rename it to examples or expo-examples,
Consider moving the example project to a folder like single-multiple-item-picker.
The examples are cool and well written. Thank you.

There are the common zIndex/overlapping issues

Can you fix them?

@taeh98
Copy link
Collaborator Author

taeh98 commented Sep 5, 2023

Hey @hossein-zare,

Please rename it to examples or expo-examples,

Okay, I'll rename the subdirectory from rndp-examples to examples.

Consider moving the example project to a folder like single-multiple-item-picker.

Do you mean to move the example source files into different folders? There are four of them, so I'm not sure if that would be useful, if that is what you mean.

The examples are cool and well written. Thank you.

You're welcome, no problem.

There are the common zIndex/overlapping issues

Can you fix them?

I'll have a go, but I'm not sure if it makes sense. Maybe it is something that should be fixed in the library: for something so simple and "out of the box", should you have to use zIndex etc to make it work normally?

@hossein-zare
Copy link
Owner

Do you mean to move the example source files into different folders? There are four of them, so I'm not sure if that would be useful, if that is what you mean.

It would be good if you create separate folders for each of them, single-item-picker and multiple-item-picker, But for now its your choice if you have time.

I'll have a go, but I'm not sure if it makes sense. Maybe it is something that should be fixed in the library: for something so simple and "out of the box", should you have to use zIndex etc to make it work normally?

Yes and https://hossein-zare.github.io/react-native-dropdown-picker-website/docs/rules#avoid-inappropriate-styles

@taeh98
Copy link
Collaborator Author

taeh98 commented Sep 5, 2023

Hey @hossein-zare,

Do you mean to move the example source files into different folders? There are four of them, so I'm not sure if that would be useful, if that is what you mean.

It would be good if you create separate folders for each of them, single-item-picker and multiple-item-picker, But for now its your choice if you have time.

I'm not sure what structure you mean. At the moment, four source files implement each example. Two are written in JavaScript and two in TypeScript. Two are class components and two are function components. They're all in the example-src-files folder and it seems reasonable to me to have four files in a folder.

I'll have a go, but I'm not sure if it makes sense. Maybe it is something that should be fixed in the library: for something so simple and "out of the box", should you have to use zIndex etc to make it work normally?

Yes and https://hossein-zare.github.io/react-native-dropdown-picker-website/docs/rules#avoid-inappropriate-styles

Yes, as in you think it should be necessary to use zIndex for a relatively simple example? Why? Surely it should be as easy to use the library as possible.

Why do you mention the Avoid inappropriate styles rule? Are you saying my examples code breaks it somewhere? It does seem restrictive to avoid using backgroundColor and borderColor, but I'm not using them in the examples anywhere, and certainly not in the containerStyle property.

@hossein-zare
Copy link
Owner

hossein-zare commented Sep 5, 2023

I'm not sure what structure you mean. At the moment, four source files implement each example. Two are written in JavaScript and two in TypeScript. Two are class components and two are function components. They're all in the example-src-files folder and it seems reasonable to me to have four files in a folder.

As an example, for the single-item-picker example keep all in the same folder, including JavaScript, TypeScript, and both function and class components.

/examples/single-item-picker/
                    single-item-picker-typescript-class.tsx
                    single-item-picker-javascript-class.jsx
                    single-item-picker-typescript-function.tsx
                    single-item-picker-javascript-function.jsx

Yes, as in you think it should be necessary to use zIndex for a relatively simple example? Why? Surely it should be as easy to use the library as possible.

No, Not a usage rule. When you encounter issues that can be fixed by using zIndex.

Why do you mention the Avoid inappropriate styles rule? Are you saying my examples code breaks it somewhere?

No, I said it generally because sometimes not using zIndex and the inappropriate styles can produce similar behaviors.

@taeh98
Copy link
Collaborator Author

taeh98 commented Sep 5, 2023

@hossein-zare

No, Not a usage rule. When you encounter issues that can be fixed by using zIndex.
No, I said it generally because sometimes not using zIndex and the inappropriate styles can produce similar behaviors.

Ah ok, I see what you mean! I think the PR is now ready to be merged. If you disagree and think more changes should be made, please let me know :)

@hossein-zare
Copy link
Owner

Yes, mergeable.
Thanks. 😁

@taeh98 taeh98 merged commit 142fdea into hossein-zare:dev-5.x Sep 5, 2023
4 of 6 checks passed
@taeh98 taeh98 deleted the examples branch September 5, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants